fix(job-orchestration): Move garbage collector recovery file to temporary directory (fixes #1823). by junhaoliao · Pull Request #1834 · y-scope/clp

junhaoliao · 2025-12-26T12:16:35Z

Description

This PR moves the garbage collector recovery file from the logs directory to the temporary directory, addressing the issues raised in #1823.

The archive_garbage_collector.py was writing its recovery file to clp_config.logs_directory, which:

Violated the convention of writing temporary files to temporary directories
Caused issues because the garbage-collector service didn't have a writable logs directory mounted

This PR:

Changed the recovery file location from clp_config.logs_directory to clp_config.tmp_directory
Added the *volume_clp_tmp mount to the garbage-collector service in Docker Compose
Fixed the Helm chart garbage-collector deployment to mount the tmp volume at /var/tmp

Checklist

The PR satisfies the contribution guidelines.
This is a breaking change and that has been indicated in the PR title, OR this isn't a
breaking change.
Necessary docs have been updated, OR no docs need to be updated.

Validation performed

Verified that the recovery file path now uses clp_config.tmp_directory
Confirmed that the garbage-collector service has the tmp volume * mounted in Docker Compose
Verified the Helm chart mounts the tmp volume at the correct path /var/tmp

Docker Compose deployment

task && cd build/clp-package && ./sbin/start-clp.sh

2026-01-14T23:21:12.117 INFO [controller] Started CLP.

All services started successfully including garbage-collector with the tmp volume mounted.

Helm chart deployments

Single-node setup

cd tools/deployment/package-helm && ./set-up-test.sh

pod/test-clp-garbage-collector-6bd59567d7-8b54g condition met
...
All jobs completed and services are ready.

Multi-node setup with shared worker nodes

cd tools/deployment/package-helm && ./set-up-multi-shared-test.sh

pod/test-clp-garbage-collector-6bd59567d7-875ws condition met
...
All jobs completed and services are ready.

Multi-node setup with dedicated worker nodes

cd tools/deployment/package-helm && ./set-up-multi-dedicated-test.sh

pod/test-clp-garbage-collector-6bd59567d7-fccqm condition met
...
All jobs completed and services are ready.

Summary by CodeRabbit

Release Notes

Chores
- Bumped Helm chart version to 0.1.4-dev.3.
- Adjusted temporary file storage location for the garbage collection service to use the system temp directory (*.tmp).
- Updated deployment manifests and compose config to mount the tmp volume at the new path (/var/tmp) across environments.

To see the specific tasks where the Asana app for GitHub is being used, see below:
- https://app.asana.com/0/0/1212888460154406
- https://app.asana.com/0/0/1212888430394537

…rary directory (fixes y-scope#1823).

… Compose.

coderabbitai · 2025-12-26T12:16:41Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2c0aa5f and df52e4f.

📒 Files selected for processing (2)

tools/deployment/package-helm/Chart.yaml
tools/deployment/package/docker-compose-all.yaml

Walkthrough

The archive garbage collector's recovery file location is changed from the logs directory to the temporary directory. Deployment manifests (Helm and Docker Compose) are updated to mount the tmp volume at the new path, and the Helm chart version is incremented.

Changes

Cohort / File(s)	Summary
Archive garbage collector code `components/job-orchestration/job_orchestration/garbage_collector/archive_garbage_collector.py`	Recovery file path moved from logs directory to temporary directory; no logic or control-flow changes.
Helm deployment template `tools/deployment/package-helm/templates/garbage-collector-deployment.yaml`	`tmp` volume mountPath changed from `/var/log` to `/var/tmp`.
Docker Compose configuration `tools/deployment/package/docker-compose-all.yaml`	Added `volume_clp_tmp` to the garbage_collector service's volume mounts.
Helm chart metadata `tools/deployment/package-helm/Chart.yaml`	Chart version bumped from `0.1.4-dev.2` to `0.1.4-dev.3`.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

Move archive_garbage_collector.py temporary file writing from /var/log to temporary directory #1823: Matches the change moving the archive garbage collector's recovery file from /var/log to a temporary directory and corresponding deployment mount updates.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: moving the garbage collector recovery file from the logs directory to the temporary directory, which is the primary objective of the PR.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

tools/deployment/package-helm/Chart.yaml

# Conflicts: # tools/deployment/package-helm/Chart.yaml # tools/deployment/package-helm/templates/garbage-collector-deployment.yaml

junhaoliao · 2026-01-14T22:32:40Z

Since "using tmpfs instead of binding mount" is a different requirement from changing the GC file list directory to use clpConfig.tmp_directory, i believe that change should be submitted in a separate PR.

junhaoliao added 2 commits December 26, 2025 04:32

fix(job-orchestration): Move garbage collector recovery file to tempo…

e65a8b4

…rary directory (fixes y-scope#1823).

fix(clp-package)!: Use tmpfs for temporary directory mounts in Docker…

958a655

… Compose.

junhaoliao mentioned this pull request Dec 26, 2025

WIP - feat(deployment): Add k8s Helm chart for CLP package deployment. #1603

Draft

16 tasks

chore(helm): bump chart version to 0.1.2-dev.18

ce02789

junhaoliao commented Jan 7, 2026

View reviewed changes

tools/deployment/package-helm/Chart.yaml Outdated Show resolved Hide resolved

junhaoliao and others added 2 commits January 7, 2026 15:22

bump chart version

8e68200

Merge branch 'main' into docker-compose-tmp-tmpfs

8dd3902

# Conflicts: # tools/deployment/package-helm/Chart.yaml # tools/deployment/package-helm/templates/garbage-collector-deployment.yaml

revert tmpfs change

2c0aa5f

junhaoliao changed the title ~~fix(clp-package)!: Use tmpfs for temporary directory mounts in Docker Compose.~~ fix(job-orchestration): Move garbage collector recovery file to temporary directory (fixes #1823). Jan 14, 2026

junhaoliao requested a review from kirkrodrigues January 14, 2026 23:31

junhaoliao marked this pull request as ready for review January 14, 2026 23:31

junhaoliao requested a review from a team as a code owner January 14, 2026 23:31

Merge branch 'main' into docker-compose-tmp-tmpfs

ba3cf5d

junhaoliao modified the milestones: Backlog, February 2026 Jan 19, 2026

Merge branch 'main' into docker-compose-tmp-tmpfs

df52e4f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(job-orchestration): Move garbage collector recovery file to temporary directory (fixes #1823).#1834

fix(job-orchestration): Move garbage collector recovery file to temporary directory (fixes #1823).#1834
junhaoliao wants to merge 8 commits intoy-scope:mainfrom
junhaoliao:docker-compose-tmp-tmpfs

junhaoliao commented Dec 26, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 26, 2025 •

edited

Loading

Uh oh!

Uh oh!

junhaoliao commented Jan 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

junhaoliao commented Dec 26, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Validation performed

Docker Compose deployment

Helm chart deployments

Single-node setup

Multi-node setup with shared worker nodes

Multi-node setup with dedicated worker nodes

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai bot commented Dec 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Uh oh!

Uh oh!

junhaoliao commented Jan 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

junhaoliao commented Dec 26, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 26, 2025 •

edited

Loading